Skip to content

Support running executables from Bundler gems and project-local scripts#141

Merged
jkutner merged 5 commits intojruby:masterfrom
bjeanes:master
Apr 5, 2013
Merged

Support running executables from Bundler gems and project-local scripts#141
jkutner merged 5 commits intojruby:masterfrom
bjeanes:master

Conversation

@bjeanes
Copy link
Contributor

@bjeanes bjeanes commented Mar 8, 2013

This feature needs to be cleaned up and refactored, but that would require refactoring some other pieces. If this is merged, I'm going to do that in a separate pull request so we can have a more critical look at the effect of the refactoring, since none of the Java portions of this gem have any automated tests.

This change enables the following:

$ java -jar app.war -S script/rails console development
Loading development environment (Rails 3.2.12)
irb(main):001:0>

It builds on the work from #130 and #136.

@kares
Copy link
Member

kares commented Mar 8, 2013

excellent -I'm really glad java -jar -S gets "finished", would be great if it still worked without Bundler as well ...

@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 8, 2013

I plan to test it without bundler too. 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to make this change to get the *.java files in master branch to compile but it is not directly related to this change. Let me know if you'd like me to break that out as a separate PR.

Bo Jeanes added 2 commits March 11, 2013 09:42
init.rb expects $servelet_container to be set. Luckily, it uses ||= to
set those env variables. To move forward, I'm pre-setting them so that
$servelet_container isn't used (since it is not set), but this
could/should be done much nicer.
@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 11, 2013

@kares as far as I know this is working as expected. There aren't any automated tests that I can find around this functionality but the general expected outcomes work for me and the existing tests pass (though Travis seems to be consistently running this project, so I can't speak for the full build matrix).

I'd like to do a refactoring of the "runnable" responsibilities entirely (as well as make it available in JarMain as well as WarMain) but they are probably best served as different pull requests.

@kares
Copy link
Member

kares commented Mar 12, 2013

Hey Bo, yeah it seems all fine. There sure are no tests (besides the original code being a "draft" :)) I only did testing locally with a Rails app as well ... maybe @BanzaiMan can merge this than (refactoring to be done on another PR) ?

@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 12, 2013

@kares ok. Make sure you tested after running rake jar. I didn't commit a regenerated jar (partially because I don't like committing build artifacts and partially because I only wanted to commit the working version).

@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 13, 2013

@kares, have you seen those Travis failures before? I am not able to reproduce them locally or reason about what part of my change could have caused them to fail.

@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 13, 2013

This is littered near the failures in the output on Travis:

fatal: attempt to fetch/clone from a shallow repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

A cursory glance at the specs make it seem like it is bundling the local git repo as a gem in a test. My (still unsubstantiated) theory is that Travis has started only doing shallow clones of tested repositories to lessen network load.

@bjeanes
Copy link
Contributor Author

bjeanes commented Mar 22, 2013

bump

@richievos
Copy link

This looks extremely useful, and not having this is one of our largest pain points in our jruby deploy right now (not having a simple solution for utility servers). So I 👍 this.

@jkutner
Copy link
Member

jkutner commented Apr 5, 2013

I've fixed the ci failures. working on getting this merged now.

@bjeanes
Copy link
Contributor Author

bjeanes commented Apr 5, 2013

👏

@jkutner jkutner merged commit 3363641 into jruby:master Apr 5, 2013
@jkutner
Copy link
Member

jkutner commented Apr 5, 2013

I'd really like to see some specs for this. It would also be nice to refactor the runnable responsibilities as you describe (pretty much everyone who uses regular non-exectuable wars has this same need). I'll look into these things myself. let us know if you have any ideas for how to go about it.

@bjeanes
Copy link
Contributor Author

bjeanes commented Apr 5, 2013

I completely agree. There wasn't a real precedent for this stuff, but am happy to help out. I actually really want to do the refactoring but held back both because I didn't want this PR to be rejected due to too many changes and because of a lack of test infrastructure. If we can agree on a structure for that aspect, I'd be more than happy to help give these aspects a revamp.

@bjeanes
Copy link
Contributor Author

bjeanes commented Apr 5, 2013

Oh also @jkutner I didn't commit the compiled results (simply so as not to add repo weight until the changes were accepted). Be sure to do a compilation before any new gem builds!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants